custom_components/polestar_api/pypolestar/polestar.py (1)

Line range hint 249-286: Enhance error handling in GraphQL query execution

The current error handling could be more comprehensive to handle network-related issues and ensure proper resource cleanup.

     async def _query_graph_ql(
         self,
         query: DocumentNode,
         operation_name: str | None = None,
         variable_values: dict | None = None,
     ):
         if self.gql_session is None:
             raise RuntimeError("GraphQL not connected")
 
         self.logger.debug("GraphQL URL: %s", self.api_url)
 
         try:
             result = await self.gql_session.execute(
                 query,
                 operation_name=operation_name,
                 variable_values=variable_values,
                 extra_args={
                     "headers": {"Authorization": f"Bearer {self.auth.access_token}"}
                 },
             )
         except TransportQueryError as exc:
             self.logger.debug("GraphQL TransportQueryError: %s", str(exc))
             if (
                 exc.errors
                 and exc.errors[0].get("extensions", {}).get("code") == "UNAUTHENTICATED"
             ):
                 self.latest_call_code = 401
                 raise PolestarNotAuthorizedException(exc.errors[0]["message"]) from exc
             self.latest_call_code = 500
             raise PolestarApiException from exc
+        except httpx.ConnectError as exc:
+            self.logger.error("Failed to connect to GraphQL endpoint: %s", str(exc))
+            self.latest_call_code = 503
+            raise PolestarApiException("Failed to connect to API") from exc
+        except httpx.TimeoutException as exc:
+            self.logger.error("GraphQL request timed out: %s", str(exc))
+            self.latest_call_code = 504
+            raise PolestarApiException("Request timed out") from exc
         except Exception as exc:
             self.logger.debug("GraphQL Exception: %s", str(exc))
             raise exc

custom_components/polestar_api/config_flow.py (1)

89-102: LGTM! Good improvement in resource management.

The addition of the try-finally block ensures proper cleanup of resources by calling async_logout() regardless of the outcome. This is a good practice for managing HTTP client resources.

Consider adding a debug log before raising exceptions to help with troubleshooting.

 if found_vins := api_client.get_available_vins():
     _LOGGER.debug("Found %d VINs for %s", len(found_vins), username)
 else:
     _LOGGER.warning("No VINs found for %s", username)
+    _LOGGER.debug("Available VINs: %s", found_vins)
     raise NoCarsFoundException

 if vin and vin not in found_vins:
     _LOGGER.warning("VIN %s not found for %s", vin, username)
+    _LOGGER.debug("Available VINs: %s", found_vins)
     raise VinNotFoundException

custom_components/polestar_api/pypolestar/auth.py (1)

60-76: LGTM! Comprehensive logout implementation.

The logout implementation thoroughly cleans up all resources including cookies and tokens. The domain-aware cookie cleanup is a good practice.

Consider adding a debug log for token cleanup similar to cookie cleanup.

 self.access_token = None
 self.id_token = None
 self.refresh_token = None
+self.logger.debug("Cleared all tokens")
 self.token_lifetime = None
 self.token_expiry = None

custom_components/polestar_api/polestar.py (1)

Line range hint 165-187: LGTM! Comprehensive error handling with appropriate retry intervals.

The error handling is well-structured with different retry intervals based on the type of error. Consider adding constants for the retry intervals to make them more maintainable.

+# Constants for retry intervals
+RETRY_INTERVAL_SHORT = 5  # seconds
+RETRY_INTERVAL_MEDIUM = 15  # seconds
+RETRY_INTERVAL_LONG = 60  # seconds

 except PolestarApiException as e:
     _LOGGER.warning("API Exception on update data %s", str(e))
-    self.polestar_api.next_update = datetime.now() + timedelta(seconds=5)
+    self.polestar_api.next_update = datetime.now() + timedelta(seconds=RETRY_INTERVAL_SHORT)
 except PolestarAuthException as e:
     _LOGGER.warning("Auth Exception on update data %s", str(e))
     await self.polestar_api.auth.get_token()
-    self.polestar_api.next_update = datetime.now() + timedelta(seconds=5)
+    self.polestar_api.next_update = datetime.now() + timedelta(seconds=RETRY_INTERVAL_SHORT)

custom_components/polestar_api/pypolestar/polestar.py (2)

93-94: Add error handling to async_logout method

While the implementation is clean and follows single responsibility, consider adding error handling to ensure proper cleanup even if the auth logout fails.

 async def async_logout(self) -> None:
-    await self.auth.async_logout()
+    try:
+        await self.auth.async_logout()
+    except Exception as exc:
+        self.logger.warning("Failed to logout: %s", str(exc))
+    finally:
+        # Ensure cleanup of local session state
+        self.gql_session = None

---

Line range hint 36-39: Refactor client management to align with PR objectives

The current implementation uses a single shared httpx client for all operations. To meet the PR objectives:

1. Separate authentication and API clients
2. Implement cookie cleanup after credential testing
3. Create dedicated clients per API interaction

Consider this architectural approach:

class PolestarApi:
    def __init__(
        self,
        username: str,
        password: str,
        auth_client: httpx.AsyncClient | None = None,
        vins: list[str] | None = None,
        unique_id: str | None = None,
    ) -> None:
        """Initialize the Polestar API."""
        # Dedicated client for auth with cookie management
        self.auth_client = auth_client or httpx.AsyncClient()
        # Separate client for API calls
        self.api_client = httpx.AsyncClient()
        self.auth = PolestarAuth(username, password, self.auth_client, unique_id)

This separation allows for:

• Cookie cleanup after auth operations
• Independent client lifecycle management
• Better alignment with the PR objectives

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

+# Cookies set by the Polestar Identity Provider that need to be cleared during logout
 OIDC_COOKIES = ["PF", "PF.PERSISTENT"]

#!/bin/bash
# Description: Verify the OIDC cookie names in the authentication response
# Expected: Response headers should contain Set-Cookie with PF and PF.PERSISTENT

curl -I -s "${OIDC_PROVIDER_BASE_URL}/as/authorization.oauth2" \
  -H "Accept: application/json" | grep -i "set-cookie"

#!/bin/bash
# Let's check the codebase for any references to these cookie names
# and their usage in authentication/logout flows

# Search for cookie names in the codebase
echo "=== Searching for PF cookie references ==="
rg "PF\." -A 2 -B 2

echo -e "\n=== Searching for cookie-related code ==="
ast-grep --pattern 'cookie'

echo -e "\n=== Checking auth/logout implementation ==="
fd "auth|login|logout" --type f --exec cat {}